Pr/refacto into dev 1#497
Conversation
Move interpolated alert/notice strings to fr.yml with English key paths. Add flash.generic.error_detail, payment and membership failure keys, subscriptions upgrade notices, account_claims rescue messages. Internationalize Admin::UserCreationForm invalid-data failure. Populate en.yml for new keys and set i18n fallbacks en -> fr. Enable raise_on_missing_translations in development. Update request and form specs to assert via I18n. Made-with: Cursor
Replace remaining Invalid data service literals with shared I18n keys, move admin breadcrumbs and helper/form user-facing strings to locale files, internationalize shared navigation/footer/layout and selected admin/public templates, and add an i18n parity rake task plus fallback documentation updates. Made-with: Cursor
Remove the default template hello key and add all remaining missing English entries so i18n:check_keys reports zero missing keys in both locales while preserving existing key structure. Made-with: Cursor
…sition - Added `test/**/*` to RuboCop exclusions in `.rubocop.yml`. - Updated README to clarify that the project now uses RSpec exclusively, removing references to legacy Minitest. - Adjusted testing documentation to reflect the removal of Minitest and the focus on RSpec, including updated commands for running tests and linting.
…Rails cops - Enabled Bundler/DuplicatedGem to detect duplicate gems in the Gemfile. - Added several performance-related cops: Performance/RedundantEqualityComparisonBlock, Performance/CollectionLiteralInLoop, Performance/MapMethodChain, Performance/Sum, Performance/Detect. - Enabled Rails cops: Rails/Pluck, Rails/WhereEquals, Rails/Present, and Rails/SkipsModelValidations for improved code quality checks.
- Migration + AttachUserToPerson; merge/link/member/account-claim flows - Annotate intentional update_columns; classify SkipsModelValidations in docs/rubocop Made-with: Cursor
- Clarified the relationship between `User` and `Person` in the README. - Revised RuboCop command descriptions in testing documentation for better understanding. - Updated audit notes on `risky_bypass` and `identity_invariant` for improved project compliance.
- Updated the last verification date across multiple documents to 2026-05-01. - Clarified the relationship between `User` and `Person`, emphasizing that every `User` is linked to a `Person` and that a `Person` can exist without a `User`. - Enhanced descriptions of the `Donation` model and its representation in the system, ensuring alignment with the current codebase and migration plans. - Improved the overall structure and clarity of the domain model, glossary, payments, and architecture documents to reflect recent changes and maintain consistency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d7d9edb4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| person = MigrationPerson.create!( | ||
| first_name: "Web", | ||
| last_name: "User", | ||
| email: user.email_address | ||
| ) |
There was a problem hiding this comment.
This migration always creates a new people row for each users.person_id IS NULL record, using user.email_address directly. On databases that already contain an unlinked Person with that same email (a common legacy state during person/user reconciliation), MigrationPerson.create! will hit the unique email index and abort the migration, blocking deployment before person_id can be enforced as non-null.
Useful? React with 👍 / 👎.
| merge_result = People::AccountMerger.new( | ||
| source_person: previous_person, | ||
| target_person: target_person, |
There was a problem hiding this comment.
cleanup_source_person merges into target_person (the raw accessor) instead of the resolved target used earlier in call. If the service is invoked via the supported ID-based API (target_person_id without target_person) and destroy_source_person: true, this passes nil to AccountMerger, causing validation failure and rolling back the link workflow for that path.
Useful? React with 👍 / 👎.
1 participant
No description provided.